The code is much more complicated. Could we have kept the previous code, and worked of DynamicColumnHandle instead of DynamicColumn and gotten the same benefit?

The columns need to be loaded to access things like min max values. The alternative would be to change the code so we can access only the metadata with something like open_metadata

We can lighten the work of group_columns_for_merge and change it so that it does not:

• filter out columns that end up being empty.
• identify the resulting ColumnType.

It would just stops at returning a

fn group_columns_for_merge(
    columnar_readers: &[&ColumnarReader],
    required_columns: &[(String, ColumnType)],
    merge_row_order: &MergeRowOrder,
) -> io::Result<BTreeMap<(String, ColumnTypeCategory), Vec<Option<DynamicColumnHandle>>>> {

On the callsite, we would then iterate through these handles, and open the DynamicColumn,
and do the conversion of the (ColumnTypeCategory, Vec<Option<DynamicColumnHandle>>) into a
(ColumnType, Vec<Option<DynamicHandle>>) where all of the columns that are Some contain at least one element.

The rest is then similar to what exists today.
The code should not be more complicated than it is today.

One tricky part to consider is how to deal with required columns.
If all columns for this (String, ColumnTypeCategory) are empty (following deletes), whether or not we want a column depends on whether the column is marked as required or not.

In the high cardinality num column case, the BTreeMap variant may cost more than 100MB heap allocation, which is why I opted to the Iterator version.
From the code complexity the main difference is the mixing of the required columns into the (sorted by column name) Column stream instead of simply inserting them into the BTreeMap

This is too complex. Can you go with the BTreeMap solution?

I switched to a BtreeMap